Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUGFIX: Support nested fluid variables by using the original fluid method #1478

Closed
wants to merge 5 commits into from
Closed

BUGFIX: Support nested fluid variables by using the original fluid method #1478

wants to merge 5 commits into from

Conversation

vertexvaar
Copy link
Contributor

@vertexvaar vertexvaar commented Dec 10, 2018 •
edited by kdambekalns

With this patch it's possible to use the fluid variable nesting feature as well as a specific accessor per element on the path.

How to use it

Create a fluid template with nested variables that access an array value:

{config.{type}.value.{flavor}}

Assign an array and the two keys:

$this->view->assign('type', 'thing');
$this->view->assign('flavor', 'yellow');
$this->view->assign('config', ['thing' => ['value' => ['yellow' => 'Okayish']]]);

@vertexvaar
Copy link
Contributor Author

vertexvaar commented Dec 10, 2018 •
edited

Whoops, there's a commit that should not be there. I gonna remove it.

@kdambekalns
Copy link
Member

Please provide a proper PR title and import the TYPO3Fluid namespaces (using an alias, if needed).

@vertexvaar vertexvaar changed the title 5.0 BUGFIX: Support nested fluid variables by using the original fluid method Dec 12, 2018
@vertexvaar
BUGFIX: Support nested fluid variables by using the original fluid me…
ce4fab4 
…thod

Neos.FluidAdaptor blocks the nested-variables-feature of fluid by overriding
the implementing method. This change relies on the parent method for path
resolving but still sets ObjectAccess as default accessor and supports
bool casting and the TemplateObjectAccessInterface interface.
All other parts are untouched. As a side effect you can now use the $accessors
parameter of the gtePath method is now usable.

fixes: neos#1477
albe
albe reviewed Dec 14, 2018
View reviewed changes
Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good codewise. Now if we could have a test to verify the fix, that would be great.

@stale
Copy link

stale bot commented May 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions!

@stale stale bot added the Stale Issues and PRs with no recent activity, about to be closed soon. label May 30, 2019
@albe
Copy link
Member

albe commented May 30, 2019

Though stale, still relevant.

@stale
Copy link

stale bot commented Jun 13, 2019

This issue has been automatically closed because it has not had activity for some time. But that does not need to be final. If you find the time to work on it, feel free to open it again. Thanks again for your contributions!

@stale stale bot closed this Jun 13, 2019
@albe albe reopened this Mar 20, 2021
@albe
Copy link
Member

albe commented Mar 20, 2021

Would like to update this and sometime actually get this in.

@albe albe changed the base branch from 5.0 to 5.3 March 20, 2021 10:54
@albe albe removed I: WIP Stale Issues and PRs with no recent activity, about to be closed soon. labels Mar 20, 2021
@albe
Copy link
Member

albe commented Mar 20, 2021 •
edited

🤞 this should pass with the added test and then this is IMO good to go - finally

Edit: Of course it doesn't pass - Failed asserting that null is identical to 'Okayish'.
Edit2: Ah yes, TYPO3/Fluid#471 (comment) still applies - Fluid can nest variables, but only one level and only one variable. Pretty limited, but that's the way it is.

@albe
Copy link
Member

albe commented Mar 20, 2021

I still think this small regex fix could have landed in Fluid - it still doesn't allow deeply nested variables, but the case in the test should be supported.

@albe
Copy link
Member

albe commented Mar 20, 2021

ping @kitsunet @kdambekalns

@kdambekalns
Copy link
Member

Merging a higher branch into a lower branch sounds very wrong and dangerous – is this what you intended, @albe?

Ah, probably because the patch comes from a 5.0 branch on th fork but the base was changed. So this is correct, but highly confusing. 👍

@albe
Copy link
Member

albe commented Apr 4, 2021

Rebased in #2444 to keep the history clean. If we had rebase & merge enabled we could solve it that way too :)

@albe albe closed this Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albe albe albe approved these changes

@kitsunet kitsunet Awaiting requested review from kitsunet

@kdambekalns kdambekalns Awaiting requested review from kdambekalns

Assignees
No one assigned
Labels
P: Fluid(Adaptor) T: PHP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@vertexvaar @kdambekalns @albe